Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tileSize for non-geospatial use-case #5886

Closed
wants to merge 2 commits into from

Conversation

ilan-gold
Copy link
Collaborator

For #5811 - this PR solves the problem by basically reverting #4616 for the non-geospatial use-case. As I mentioned, though, the only problem now is that I have come to rely on the ability to change the zoom level at which tiles are fetched in the non-geospatial case.

Should I follow this PR up with another that allows the user to offset the zoom level in the non-geospatial instance? Should this PR not revert the changes from #4616 and just document what people need to do with regards to the tileSize prop when they are using the TileLayer in the non-geospatial use-case.

Change List

@coveralls
Copy link

coveralls commented Jun 16, 2021

Coverage Status

Coverage increased (+0.005%) to 82.354% when pulling 0329f45 on ilan-gold:infovis_tile_offset into 732a1c0 on visgl:master.

@ilan-gold ilan-gold changed the title Add back tileSize to utils functions. Fix tileSize for non-geospatial use-case Jun 16, 2021
@Pessimistress
Copy link
Collaborator

Can you review the indexing system and tileSize sections of the documentation and see if this change can be explained in a way that is clear and (ideally) inconsistent with the geospatial use case? Can we just reference the DZI spec for non-geospatial the same way we use OSM?

Where tileSize does not intuitively define the desired "zoom offset", can we expose something else for the user to customize the layer behavior?

@ilan-gold
Copy link
Collaborator Author

@Pessimistress Sounds like a plan. I'll add the documentation for this PR and open another for the new prop then to control the "zoom offset."

@ilan-gold
Copy link
Collaborator Author

Closing in favor of #5895

@ilan-gold ilan-gold closed this Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants